Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Random rotation #16794

Merged
merged 8 commits into from
Feb 11, 2020
Merged

Random rotation #16794

merged 8 commits into from
Feb 11, 2020

Conversation

lkubin
Copy link
Contributor

@lkubin lkubin commented Nov 12, 2019

Description

  • We implemented functions for image rotation and random rotation in mxnet.image.image.py, a transform for Rotation and one for RandomRotation in mxnet.gluon.data.vision.transforms.py

Checklist

Essentials

  • The PR refers to the relevant issues [Feature Request] Random Rotation Image Augmentation #13206 Random Rotation Image Augmentation #11230
  • Changes are complete
  • All changes have test coverage
  • Unit tests are added for both mxnet.image.image.py functions and for mxnet.gluon.data.vision.transforms.py
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Image rotation function imrotate, based on BilinearSampler function. Tested in tests/python/unittest/test_image.py
  • transforms.Rotate, tested in tests/python/unittest/test_gluon_data_vision.py
  • Image random rotation function random_rotate. Tested in tests/python/unittest/test_image.py
  • transforms.RandomRotation, tested in tests/python/unittest/test_gluon_data_vision.py

Comments

All the implemented functionalities are based on imrotate function. The shape of the output of this function is the same shape of the input, thus it can be used inside a transformer.
It supports 3 behaviors:

  • Simple image rotation without zooming
    alt text
  • Zoom in image rotation: no padding is shown in the output result
    alt text
  • Zoom out image rotation: the whole original image will be contained in the output result
    alt text

@lkubin lkubin requested a review from szha as a code owner November 12, 2019 17:06
@lkubin
Copy link
Contributor Author

lkubin commented Nov 13, 2019

The validation on windows failed during the build... I think it might need to be restarted.

Added function for image rotation (imrotate) using BilinearSampler
0e68b29 
Added unit tests for imrotate

Added Rotate to transforms

Added RandomRotation to transforms
Added transforms tests
@lkubin
Copy link
Contributor Author

lkubin commented Jan 16, 2020

@szha I opened this pull request 2 moths ago, I'm wondering if it is an useful feature to be added. Otherwise I'm going to close it.

@marcoabreu
Copy link
Contributor

This is a very useful feature, thanks!

@reminisce @eric-haibin-lin @apeforest could you review please?

@samskalicky
Copy link
Contributor

@lkubin Can you please rebase/merge to fix the conflicts?

guanxinq
guanxinq reviewed Jan 21, 2020
View reviewed changes
python/mxnet/gluon/data/vision/transforms.py Outdated Show resolved Hide resolved
python/mxnet/gluon/data/vision/transforms.py Outdated Show resolved Hide resolved
python/mxnet/gluon/data/vision/transforms.py Outdated Show resolved Hide resolved
python/mxnet/gluon/data/vision/transforms.py Show resolved Hide resolved
python/mxnet/image/image.py Show resolved Hide resolved
tests/python/unittest/test_gluon_data_vision.py Show resolved Hide resolved
zhreshold
zhreshold approved these changes Jan 21, 2020
View reviewed changes
Copy link
Member

@zhreshold zhreshold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me! Can you resolve the conflicts?

@lkubin
Copy link
Contributor Author

lkubin commented Jan 24, 2020

We solved the conflicts and we did the changes that you were suggesting. Let us know if we miss something!

guanxinq
guanxinq reviewed Jan 24, 2020
View reviewed changes
Copy link
Contributor

@guanxinq guanxinq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for your contribution.

@samskalicky
Copy link
Contributor

@mxnet-label-bot add [pr-awaiting-merge]

@lanking520 lanking520 added the pr-awaiting-merge Review and CI is complete. Ready to Merge label Jan 24, 2020
@zhreshold
Copy link
Member

@lkubin Seems like the ci is consistently failing on windows machines, can you rebase to master and I can look into the problem if it's still failing

@lkubin
Copy link
Contributor Author

lkubin commented Jan 28, 2020

@zhreshold I just merged master into this branch. I'm not sure what's happening with the CI on windows, it seems failing during compiling: I don't think the failure is related to this PR.

@lkubin
Copy link
Contributor Author

lkubin commented Jan 28, 2020

@zhreshold I see that tests on windows are still failing. I cannot understand why though.

@lkubin
Copy link
Contributor Author

lkubin commented Jan 29, 2020 •
edited
Loading

@zhreshold it seems that also the CI on ubuntu on GPU is failing... the process got killed with SIGTERM, maybe a timer expired?

@zhreshold
Copy link
Member

Restarted the jobs, let's wait for another round

@lkubin
Copy link
Contributor Author

lkubin commented Jan 31, 2020

@zhreshold It seems that now only linux GPU tests are not passing. Can you restart only those tests to see if they will pass the CI?

@samskalicky
Copy link
Contributor

restarted

@lkubin
Copy link
Contributor Author

lkubin commented Feb 1, 2020

Guys, the CI for Unix still failing, this time with RuntimeError: Python 3.5 or later is required during the installation of pip. Is there something I can do in order to make it work?

@zhreshold
Copy link
Member

I think someting is broken in CI which is related to recent deprecation of python2.7

@zhreshold
Copy link
Member

@lkubin I think the ci python3.5 error is fixed: #17457

Can you do rebase to master one more time?

@lkubin
Copy link
Contributor Author

lkubin commented Feb 6, 2020

@zhreshold I think that now the build for R is failing...

@samskalicky
Copy link
Contributor

samskalicky commented Feb 6, 2020 •
edited
Loading

@zhreshold I think that now the build for R is failing...

@lkubin we had to restart the CI last night. Ive restarted the last two CI runs. Hopefully they'll pass now!

@zhreshold zhreshold merged commit 6c61afb into apache:master Feb 11, 2020
@zhreshold
Copy link
Member

All green, merged! Thanks guys 😸

zheyuye pushed a commit to zheyuye/incubator-mxnet that referenced this pull request Feb 19, 2020
@lkubin @douglas125
Random rotation (apache#16794)
0621a83 
* Added function for image rotation (imrotate) using BilinearSampler

Added unit tests for imrotate

Added Rotate to transforms

Added RandomRotation to transforms
Added transforms tests

* Made rotations generic - any angle

* Added tests. Removed useless docstrings. Updated API.

Co-authored-by: Douglas Andrade  <[email protected]>

Co-authored-by: Douglas Coimbra de Andrade <[email protected]>
anirudh2290 pushed a commit to anirudh2290/mxnet that referenced this pull request May 29, 2020
@lkubin @douglas125 @anirudh2290
Random rotation (apache#16794)
eab87f5 
* Added function for image rotation (imrotate) using BilinearSampler

Added unit tests for imrotate

Added Rotate to transforms

Added RandomRotation to transforms
Added transforms tests

* Made rotations generic - any angle

* Added tests. Removed useless docstrings. Updated API.

Co-authored-by: Douglas Andrade  <[email protected]>

Co-authored-by: Douglas Coimbra de Andrade <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@guanxinq guanxinq guanxinq approved these changes

@douglas125 douglas125 douglas125 approved these changes

@marcoabreu marcoabreu marcoabreu approved these changes

@zhreshold zhreshold zhreshold approved these changes

@szha szha Awaiting requested review from szha szha is a code owner

Assignees
No one assigned
Labels
pr-awaiting-merge Review and CI is complete. Ready to Merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@lkubin @marcoabreu @samskalicky @zhreshold @douglas125 @guanxinq @lanking520